Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
df125fc to
b021c58
Compare
5706e24 to
2413765
Compare
…/registry scaffolding - Create rubricTemplate.ts with pure functions for CRUD operations on RubricTemplateSchema (scored, yes/no, dropdown, long_text criterion types) - Extend FieldConfigCard with optional collapsible/accordion behavior using React Aria Disclosure primitives with animated height transitions - Add headerExtra prop to FieldConfigCard for rendering badges in the header - Add setRubricTemplateSchema/getRubricTemplateSchema to ProcessBuilder store - Create rubricCriterionRegistry with type metadata, icons, and labels
- Create RubricCriterionCard with collapsible FieldConfigCard, inline criterion type radio selector, scored/dropdown/yes-no/long-text configs - Create RubricEditorContent with Accordion+Sortable for drag-to-reorder, debounced auto-save to API, live RubricParticipantPreview, and empty state - Wire RubricEditorContent into CriteriaSection behind rubric_builder flag
Use EmptyState component with leaf icon, descriptive title/subtitle, and primary CTA button. Rename section title to 'Review Criteria'.
Only trigger auto-save after a user-initiated edit, not on mount. Use a hasUserEditedRef guard so the debounced save effect doesn't fire from dependency reference changes or initial template hydration.
This reverts commit ddf1466.
…type - Remove 'dropdown' criterion type, leaving only scored/yes_no/long_text - Rewrite RubricCriterionCard to use Accordion primitives directly (not FieldConfigCard) with static 'Criterion N' header, separate name/description fields, and type radio - Wrap criteria in AccordionItem in RubricEditorContent (matching PhasesSectionContent pattern) - Update rubricCriterionRegistry with new labels (Rating Scale, Yes/No, Text response only) - Clean up unused handlers and imports from RubricEditorContent - Add 15 new translation keys to all 5 language files
Follows the same pattern as PhasesSectionContent: clicking the trash button now opens a dismissable modal asking the user to confirm before the criterion is removed. Adds translation keys for the modal to all 5 language files.
Move the 'Criterion N' label inside AccordionTrigger so clicking anywhere on the header (except drag handle and delete button) toggles the accordion.
The accordion header now displays: 'Criterion N' (always), the criterion name in muted text (once set and not the default), a type badge (Rating Scale / Yes/No / Text response only), and a points badge for scored criteria (e.g. '5 pts'). The name truncates gracefully when the header is narrow.
Replace 'Score labels' and 'Label for score {number}' with new keys:
- 'Define what each score means'
- 'Help reviewers score consistently by describing what each point value represents'
- 'Describe what earns {number} points...'
Updated across all 5 language files (en, es, fr, pt, bn).
New scored criteria now start with empty label strings instead of 'Poor/Below Average/Average/Good/Excellent'. The placeholder text in the textareas guides the user on what to fill in.
getCriterionScoreLabels was sorting oneOf entries descending (b.const - a.const) so score 5 appeared at index 0, but updateScoreLabel writes by raw oneOf array index where score 1 is at index 0. Changed to ascending sort (a.const - b.const) so the displayed order matches the storage order.
Restore descending sort in getCriterionScoreLabels (highest score first) and change updateScoreLabel to match oneOf entries by their .const value rather than array position. The UI now derives the score value from the descending index (max - i) and passes it through, so edits land on the correct score level.
scazan
left a comment
There was a problem hiding this comment.
really nice work. The comments are a mix of comments and change requests. I think our typing is getting too loose with these templates and we want to keep them in check. We know what they look like so we should try to avoid unknown and type assertions in the frontend since new processes don't inherit legacy schemas.
The other thing is that we should be building a library of utils for dealing with templates generally since we have both rubric and proposal templates so extracting out the JSON Schema tools that are common is a good idea here already.
apps/app/src/components/decisions/ProcessBuilder/components/ConfirmDeleteModal.tsx
Outdated
Show resolved
Hide resolved
apps/app/src/components/decisions/ProcessBuilder/stepContent/rubric/CriteriaSection.tsx
Outdated
Show resolved
Hide resolved
apps/app/src/components/decisions/ProcessBuilder/stepContent/rubric/RubricCriterionCard.tsx
Outdated
Show resolved
Hide resolved
apps/app/src/components/decisions/ProcessBuilder/stepContent/rubric/RubricEditorContent.tsx
Outdated
Show resolved
Hide resolved
apps/app/src/components/decisions/ProcessBuilder/stepContent/rubric/RubricEditorContent.tsx
Outdated
Show resolved
Hide resolved
apps/app/src/components/decisions/ProcessBuilder/stepContent/rubric/RubricEditorContent.tsx
Outdated
Show resolved
Hide resolved
apps/app/src/components/decisions/ProcessBuilder/stepContent/rubric/RubricEditorContent.tsx
Show resolved
Hide resolved
apps/app/src/components/decisions/ProcessBuilder/stepContent/rubric/RubricEditorContent.tsx
Outdated
Show resolved
Hide resolved
- Create templateUtils.ts with generic operations shared between rubric and proposal templates - Refactor rubricTemplate.ts to delegate to shared utils - Refactor proposalTemplate.ts to delegate to shared utils - Removes code duplication for property readers and mutators
- Create rubricTemplateEncoder Zod schema for the read path - Provides proper typing for instanceData.rubricTemplate - Keep jsonSchemaEncoder for write path to maintain compatibility - Update test to work with new encoder types
- Move component from ProcessBuilder-specific to shared @/components - Remove 'use client' directive (not needed, parent is client component) - Delete old re-export file as all imports updated
- Create RubricEditorSkeleton for loading state - Add ErrorMessage fallback to ErrorBoundary - Add RubricEditorSkeleton fallback to Suspense
- Replace instance.status === 'draft' with ProcessStatus.DRAFT - Update imports to use @/ path aliases - Affects 5 ProcessBuilder section files
- Use @/ path aliases for imports - Add explicit border-neutral-gray1 to hr separator - Simplify padding: px-4 pt-4 pb-4 to p-4, px-3 py-3 to p-3 - Remove max points upper limit of 10, add error message for min 2 - Add 'Minimum is 2' translation to all locales
- Type scoredConfigCacheRef with { const: number; title: string }[]
- Remove 'as' type casts by using proper type guards
- Use useShallow from zustand for destructured store selectors
- Switch criteriaIndexMap to 0-based, add +1 at display points
- Remove manual teal color overrides on secondary button
- Add TODO for useAutoSave hook extraction
- Use @/ path aliases and ProcessStatus enum
- Cache score label descriptions when maxPoints decreases so they can be restored if the user increases maxPoints again - Cache persists only while the criterion card is mounted - Uses onChange (responsive) instead of onBlur since caching prevents data loss
scazan
left a comment
There was a problem hiding this comment.
Approving since we can ship without it but we should file an issue if we don't fix it with the encoder types (then we can do a separate PR to sort through the rubric types). Ideally let's try to fix it first but it won't cause bugs.. yet. :) I'm worried the types are getting very loose with these two templates when they don't need to be (and we can even enforce these types at the DB level when the dust settles a bit).
Nice with the util extractions and other fixes!
apps/app/src/components/decisions/ProcessBuilder/stepContent/rubric/RubricEditorContent.tsx
Show resolved
Hide resolved
| template: RubricTemplateSchema, | ||
| criterionId: string, | ||
| ): XFormatPropertySchema | undefined { | ||
| return getPropertySchema(template, criterionId); |
There was a problem hiding this comment.
Any reason for these wrappers? We should be able to use the utils directly now in many parts of this file.
There was a problem hiding this comment.
Also going to handle this in a separate PR since it touches both rubricTemplate.ts and proposalTemplate.ts
| required: z.array(z.string()).optional(), | ||
| 'x-field-order': z.array(z.string()).optional(), | ||
| }) | ||
| .passthrough(); |
There was a problem hiding this comment.
Is this passthrough necessary? I think it's causing the type assertion in the frontend because it resolves to [x: string]: unknown.
Then I also wonder if we can update packages/common/src/services/decision/types.ts:36 to use the type from this encoder.
There was a problem hiding this comment.
Claude was worried that because rubric templates are JSON Schema objects, and JSON Schema has many optional fields ($schema, $id, definitions, etc.) that we don't explicitly list, we would need the passthrough to not lose any data.
Going to handle this in a separate PR since there are shared types with the proposal builder and I want to update them together.
Summary
Adds the rubric editor UI to the Process Builder, allowing process owners to define review criteria (scored, yes/no, text response) that reviewers use to evaluate proposals.
What's included
rubricTemplate.ts) — pure functions for creating, reading, and immutably updating rubric JSON Schema, with typed helpers to eliminate boilerplateLuLeaficon)rubric_builderfeature flagNot included
Testing
Use the preview deployment. Make sure you have a phase with "Enable proposal review" toggled on.